Fix: make the docs build strict without suppressions - #1546
Conversation
Two gaps left the docs build weaker than it looked. A page absent from nav still built cleanly under --strict, showing up as one INFO line. So the rule docs/README.md states — a new doc needs a nav entry — was unenforceable, and a missing entry silently dropped the page out of the site's navigation. validation.nav.omitted_files now warns, which --strict treats as a failure. Verified both ways: a page left out of nav fails the build, and removing it passes again. Twenty-six existing pages are out of nav on purpose: the investigations log, the remote-L3 design set, and the per-code error pages are reached through their own index pages, and listing every entry would bury the rest of the tree. They are declared in not_in_nav so the check stays strict for everything else rather than being switched off. The nine parameters that griffe reported as documented-but-unannotated are annotated, so docs/_hooks/griffe_filter.py — which existed only to keep that one message from failing --strict — is deleted. The build now passes --strict with zero warnings and nothing suppressed. Two of those annotations needed a decision rather than a transcription. ChipWorker.init's `bins` is structurally typed: the docstring accepts any object exposing the *_path attributes, and the body reads dispatcher_path through getattr, so it is Any. Naming RuntimeBinaries would be both narrower than the contract and a dependency from simpler onto simpler_setup. ChipWorker.run's `handle` is a CallableHandle, imported under TYPE_CHECKING because the runtime import is deliberately lazy at its use site; PEP 563 is already on in both files, so the annotations cost no import. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughMkDocs strict validation now reports omitted and missing navigation pages, while the Griffe filter hook is removed. Worker initialization and execution interfaces now include explicit type annotations with annotation-only imports where needed. ChangesDocumentation validation and worker typing
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Two gaps left the docs build weaker than it looked. After this,
mkdocs build --strictpasses with zero warnings and nothing suppressed.1. The nav rule was unenforceable
docs/README.md(from #1525) states that a new doc needs anaventry inmkdocs.yml. I tested that claim and it was not enforced: a page absent fromnavbuilt cleanly under--strict, surfacing as a single INFO line, so a missing entry silently dropped the page out of the site's navigation.validation.nav.omitted_files: warnnow makes--strictfail on it. Verified both ways — a page left out ofnavfails the build, and removing it passes again.26 existing pages are out of
navon purpose and are declared innot_in_navrather than switching the check off:investigations/*investigations/README.mdremote-l3-worker-design/*troubleshooting/device-error-codes/*Listing every entry would bury the rest of the tree; declaring them keeps the check strict for everything else.
2. The griffe suppression is now unnecessary
#1543 shipped
docs/_hooks/griffe_filter.pypurely to keep one message — "No type or annotation for parameter" — from failing--strict. Its own docstring said "Remove this hook once the annotations land." All nine are annotated here, so the hook is deleted. Warning count is 0 with no filter in place.Bonus: the generated signatures now carry types —
device_id: int,log_level: int | None,config: CallConfig | None— instead of bare names.Two annotations needed a decision, not a transcription
This is why I did not do it in #1543 as a mechanical pass:
ChipWorker.init'sbins→Any. The docstring accepts "any object exposing host_path / aicpu_path / …", and the body readsdispatcher_paththroughgetattr. NamingRuntimeBinarieswould be both narrower than the actual contract and a dependency fromsimplerontosimpler_setup— the wrong direction perproject-layout.md.ChipWorker.run'shandle→CallableHandle, imported underTYPE_CHECKING. The runtime import is deliberately lazy at its use site (task_interface.py:1157, with a# noqa: PLC0415), so a module-level import would undo that on purpose-avoided grounds. Both files already havefrom __future__ import annotations, so the annotations cost no import at all.Verification
mkdocs build --strict→ exit 0, 0 warnings, 98 pages, withgriffe_filter.pyremovedruff check/ruff format --checkclean;test_package_surface.py5 passed / 1 skipped